-
-
Notifications
You must be signed in to change notification settings - Fork 535
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[18.0][ADD] account_payment_base_oca: New base module for OCA/bank-payment #1401
base: 18.0
Are you sure you want to change the base?
Conversation
Why this? Why didn't you answer to my comment in the issue? I don't want data model changes if they are not justified. |
I'm trying to adopt the new datamodel. I'm not saying it's the best choice, but I want to try and see what difficulties I face. |
Then we are going to have 2 forks. I don't want that "datamodel", as it's very bad as explained in the migration issue. |
0920212
to
d819284
Compare
I just pushed a new version of account_payment_base_oca. In this new version, the active field of account.payment.method.line (added by account_payment_base_oca) is set to False by default. So, when a account.payment.method is added, all the account.payment.method.line automatically created by Odoo are inactive. You can active the ones you need and leave the others inactive. |
I sincerely see it as a nonsense to have to look for "similar" things one way and the reverse because of a failed datamodel putting the data at journal level. You can't for example group by this field if you select different lines according the journal but representing the same payment mode. Let's keep our |
d819284
to
7871388
Compare
@pedrobaeza It's also a big temptation for me to keep account.payment.mode. In fact, I'm the one who introduced account.payment.mode in v9 during the Sorrento code sprint (to replace payment.mode that was dropped by Odoo SA with the removal of the module account_payment from the official addons), so it's kind of my baby and I like it. But, in the long list of wired/sub-optimal datamodel of Odoo SA that I would love to avoid/change, this one is not a big deal. I think I found a solution that is acceptable and the balance between the advantage of being compatible with the native datamodel and the drawbacks of using the trick I propose to make the new datamodel work with bank_account_link = 'variable' tend towards the adoption of the new datamodel I think. I'll continue my tests in the coming days to make sure I don't discover any other problems and I'll publish the upper-level modules. |
7871388
to
9da52f0
Compare
# the account.payment but it will select the (inactive) method line | ||
# linked to the chosen journal | ||
# TODO default=False causes problems in tests of the account module | ||
active = fields.Boolean(default=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to set active=False on create only when bank_account_link is variable?
This sounds promising. I also prefer using the native |
I insist: how it would be better to assign at partner level the payment method line linked to a specific journal, seeing all of them in the dropdown (and this active=False trick is something conflicting with the way Odoo expects to work), instead of having an agnostic payment mode? The payment method lines are not new. They are there since ages (v14). The only new thing is that there's a many2one at partner level, but that doesn't change anything. Please don't go with this, or we will have 2 |
Switch back to active True by default
Another reason spotted now for not switching from |
@pedrobaeza I think Alexis is doing a lot of very valuable work here. Let's wait a bit to see where this goes. |
No, if the foundations are incorrect, no more work should be put in here. I announce my intention on continuing with the old stack if you switch, and the model names should be preserved for the existing things according current establishment. |
Sorry I don't understand your arguments, Pedro. Odoo is evolving and it seems normal to me to explore and see if we can adapt. |
The arguments are clear of the cons of switching, so I'm just stating that I would push for the previous approach and this will conflict in the model names, and it's clear that these cons can't be bypassed without doing ugly hacks and standard code overrides. |
…riable_journal_ids
Note that I'm the first one embracing upstream changes when they are suitable for the goal of the modules. I did the refactoring to But in this case, it has no benefit to go with a simple wrong m2o extra field that they have added in the partner, and that is so wrong in the concept. And I insist that the |
Work in progress. Expect datamodel changes.